-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Extend the list of widgets that cannot be used inside the List… #10928
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/get-appsmith/appsmith/DgpqrLY34K4ijTx3qgbzUi73m31Z |
Unable to find test scripts. Please add necessary tests to the PR. |
/ok-to-test sha=488382d |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1806265670. |
UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1806265670. Click to view performance test results
|
/ok-to-test sha=488382d |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1808086200. |
UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1808086200. Click to view performance test results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
@YogeshJayaseelan Needs a regression test for existing widget which is now disallowed. Can we play around with that state, moving the widget outside and then inside the List Widget, and check the experience.
@sbalaji1192 Tested this PR and now the below widgets are allowed inside list widget "AUDIO_WIDGET", Also, when trying to place non allowed widgets - A valid toast message appears and the list widget doesn't break. Issue 1 : Placing a List widget within list widget - the app crashes Step to Reproduce: |
/ok-to-test sha=5c465e9 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1816246593. |
UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1816246593. Click to view performance test results
|
@YogeshJayaseelan The above callout is not related to the list widget. It's a know issue and being tracked here - #10659 |
@sbalaji1192 Apart from the list widget inside another list widget issue rest of everything works fine |
… widget
Description
Only widgets that don't have derived or meta properties work well inside the current version of the List widget. Widgets like Input, Select maintain the state on meta properties which won't be available in List.selectedItem object. Hence we're restricting them from being placed inside the List widget.
Related #10683
Type of change
How Has This Been Tested?
Checklist:
Test coverage results 🧪
🔴 Total coverage has decreased